Async grpo OpenEnv harness rollout#6420
Conversation
dabfebb to
1903378
Compare
| reward = float(verify.env_reward) if verify.env_reward is not None else None | ||
| sequences = _chain_to_sequences(turns, rollout_id, self._fork_threshold_tokens) | ||
| completion_ids = [tid for turn in turns for tid in turn.output_ids] | ||
| return completion, completion_ids, sequences, tool_call_count, tool_failure_count, reward |
There was a problem hiding this comment.
Empty traces skew advantages
High Severity
When the proxy trace is empty or _trace_output_ids recovers no token ids, _chain_to_sequences yields no training rows, but session.verify still supplies a rollout_reward that enters the group's advantage normalization. Sibling generations are then trained against rewards from conversations that contribute no samples.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 284da95. Configure here.
There was a problem hiding this comment.
When the proxy trace is empty or _trace_output_ids recovers no token ids
no idea in what case this is possible
There was a problem hiding this comment.
I dont think thats real case
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
Thanks I think it looks good overall. I didn't try myself, but from reading the edits, nothing much to say. Check the cursor review, it's likely the reason of the CI failure. Once the CI green I approve and we merge |
sergiopaniego
left a comment
There was a problem hiding this comment.
thanks!!
approving but with 2 ideas:
- we could add a short harness-rollout section to
openenv.mdin this PR: white-box vs loop-owning,--return-tokens-as-token-ids, reward viasession.verify()→harness_reward. - we should add an example script. I think this could be added once we add support for HF sandbox + harnesses. wdyt? it's in the roadmap here -> huggingface/OpenEnv#940
|
Ran this branch loop-owning with OpenCode on HF Jobs; two things surfaced (both line up with the Cursor comments above, now with a concrete repro): 1. Minimal guard: choices = last["response"].get("choices") or []
content = choices[0]["message"].get("content", "") if choices else ""(plus rebuilding the final assistant 2. Rollout child doesn't exit cleanly. Confirmed the |
|
Heads-up on another rough edge hit while training a loop-owning harness (Pi coding agent) on this branch, separate from the empty-
Repro-side workaround: coerce structured content to text before append, e.g. c = msg["content"]
t.append(c if isinstance(c, str) else "".join(
p.get("text", "") if isinstance(p, dict) else str(p) for p in c))Happy to open a small PR if useful. |
|
@sergiopaniego can you check #5309, would it solve the issue? I can revamp it you think it makes sense |
@sergiopaniego thanks for catching this, but |
| # it should retry with backoff (ideally inside OpenEnv's factory). For now a failed create drops the rollout as | ||
| # unscorable, which shrinks the effective group size - a high create-failure rate silently weakens the signal. | ||
| try: | ||
| session = self._factory.create(prompt, seed=seed, episode_id=rollout_id) |
There was a problem hiding this comment.
Group seed never varies
High Severity
_HarnessRolloutLoop._generate_one takes group_id and uses it as the session seed, but the parent generate loop never passes group_id, so every rollout keeps the default 0. Seed-driven factories therefore hand every group the same task for the whole run.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 06b6083. Configure here.
| last = entries[-1] | ||
| choices = (last.get("response") or {}).get("choices") or [] | ||
| content = choices[0]["message"].get("content", "") if choices else "" | ||
| return list(last["request"].get("messages") or []) + [{"role": "assistant", "content": content}] |
There was a problem hiding this comment.
Structured content crashes logging
High Severity
Loop-owning completions keep agent message content as-is, including structured lists. With log_completions=True, print_prompt_completions_sample appends that value to rich Text and raises TypeError, which kills the score loop and stops the rollout worker.
Reviewed by Cursor Bugbot for commit 06b6083. Configure here.
|
In this commit : 06b6083 fixed @sergiopaniego you can rerun if possible :
OpenEnv Workarounds @burtenshaw @adithya-s-k @sergiopaniegoI had to work around some OpenEnv fundamental type /API limitation. I noted them as
So what I just pushed is
I also added this : cf #6349 @qgallouedec
|
qgallouedec
left a comment
There was a problem hiding this comment.
lgtm, just a bug (not sure) on group id
| # In-flight sessions, so `_run_loops` can close them on stop (see there). set ops are atomic under the GIL. | ||
| self._live_sessions: set = set() | ||
|
|
||
| async def _generate_one(self, prompt, tool_dict, tools, group_id=0): |
There was a problem hiding this comment.
group_id isn't yet wired, right?
| ) | ||
| pending_completed[group_id] = 0 | ||
|
|
||
| task = asyncio.create_task(self._generate_one(prompt, tool_dict=tool_dict, tools=tools)) |
There was a problem hiding this comment.
see https://github.com/huggingface/trl/pull/6420/changes#r3632113776, shouldn't we have group_id here?
| def _tool_failure_count(entries: list[TraceEntry]) -> int: | ||
| """Best-effort tool-failure count across the real agent turns (`entries`). The agent (not TRL) executed the tools, | ||
| so we only see their free-text RESULTS, which come back as `role="tool"` messages in a later request; a failure can | ||
| only be inferred from error-looking result text. The last agent request holds the fullest message list; dedupe by | ||
| (name, content) so a result echoed across requests is not counted twice.""" | ||
| seen, failures = set(), 0 | ||
| for msg in (entries[-1]["request"] if entries else {}).get("messages") or []: | ||
| if msg.get("role") != "tool": | ||
| continue | ||
| key = (msg.get("name"), str(msg.get("content"))[:200]) | ||
| if key in seen: | ||
| continue | ||
| seen.add(key) | ||
| text = str(msg.get("content") or "").lower() | ||
| if any(w in text for w in ("error", "failed", "traceback", "exception")): | ||
| failures += 1 | ||
| return failures |
There was a problem hiding this comment.
quite fragile, consider dropping it.
if you want to keep it, let's keep in mind that we should re-visit later when openenv exposes real tool-result status
| policy version from the shared `mp.Value` (`model_version_value`). | ||
| """ | ||
|
|
||
| # Class attribute declares that this loop produces its own per-rollout reward |
There was a problem hiding this comment.
| # Class attribute declares that this loop produces its own per-rollout reward | |
| # Class attribute: declares that this loop produces its own per-rollout reward |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9015404. Configure here.
| return [] | ||
| last = entries[-1] | ||
| choices = (last.get("response") or {}).get("choices") or [] | ||
| content = choices[0]["message"].get("content", "") if choices else "" |
There was a problem hiding this comment.
Fragile final-choice message access
Medium Severity
Empty choices is handled, but a non-empty choice missing message still does choices[0]["message"] and raises. That happens after turns are already built, so the outer handler discards an otherwise trainable rollout as unscorable.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9015404. Configure here.


WIP
Note
Medium Risk
Touches the experimental async GRPO rollout/scoring path (new reward column and return signatures) and runs untrusted agent code via local subprocess sandboxes; failures are mostly isolated but concurrent rollouts increase operational complexity.
Overview
Adds OpenEnv harness support to Async GRPO so rollouts can be driven by external agents that own their tool loop, instead of TRL’s built-in vLLM turn loop.
A new
HarnessRolloutWorker/_HarnessRolloutLoopruns OpenEnv sessions (white-box viaHarnessAdapteror loop-owning via proxy trace), rebuildsTurnRecords from captured token ids/logprobs, verifies in the sandbox, and scores via an optionalrollout_reward_fn. Hookstrain_turn_fnandagent_turn_fnfilter which trace turns become training rows. Failed sessions degrade to unscorable rollouts instead of killing the worker.Async rollout plumbing is extended:
RolloutGroup.rollout_rewards,_generate_onereturns a sixthrollout_rewardslot,_provides_rollout_rewardlets loops score withoutreward_funcs, the child process accepts a pluggable_loop_cls, andRolloutWorkerProtocolallowsmultiprocessing.Queuebuffers.The
examples/scripts/openenv/opencode.pyscript demonstrates end-to-end training:LocalSubprocessSandboxBackend(path remap from/home/user, process-group cleanup, template hardlink clones), DeepCoder held-out stdin/stdout verifier, per-session free proxy ports, and opencode-specific reward/turn filtering wired intoAsyncGRPOTrainer.Reviewed by Cursor Bugbot for commit 9015404. Bugbot is set up for automated code reviews on this repo. Configure here.